Skip to content

Add optional parameter to configuration providers for non-blocking startup#1745

Open
GeorgeTsiokos wants to merge 3 commits intodapr:masterfrom
GeorgeTsiokos:feature/optional-config-resilience
Open

Add optional parameter to configuration providers for non-blocking startup#1745
GeorgeTsiokos wants to merge 3 commits intodapr:masterfrom
GeorgeTsiokos:feature/optional-config-resilience

Conversation

@GeorgeTsiokos
Copy link

@GeorgeTsiokos GeorgeTsiokos commented Mar 13, 2026 •
edited
Loading

Summary

Resolves #1748

  • Adds an optional parameter to AddDaprConfigurationStore, AddStreamingDaprConfigurationStore, and AddDaprSecretStore extension methods so applications can start without blocking on Dapr sidecar availability
  • Hardens LoadInBackgroundAsync in both providers with narrowed exception handling (OperationCanceledException + DaprException) instead of a broad catch (Exception) that silently swallowed programming errors
  • Fixes resource leak by disposing CancellationTokenSource in both providers
  • Fixes StringComparer inconsistency (InvariantCultureIgnoreCase → OrdinalIgnoreCase) in FetchSecretsAsync
  • Makes fields readonly in DaprConfigurationStoreProvider for consistency with DaprSecretStoreConfigurationProvider
  • Restores UTF-8 BOM per .editorconfig and adds missing trailing newlines

Test plan

  • All 52 existing + new unit tests pass across net8.0, net9.0, net10.0
  • New unit tests use GetReloadToken() callbacks for deterministic assertions (no fragile Task.Delay polling)
  • Integration test: optional: true with delayed sidecar — config populates once sidecar starts (ShouldPopulateSecretsWhenSidecarStartsLate)
  • Integration test: optional: false (default) behavior unchanged — Build() blocks until sidecar ready (ShouldBlockAndPopulateSecretsWhenNotOptional)
  • Integration test: disposal during background loading exits cleanly without hanging (ShouldExitCleanlyWhenDisposedDuringBackgroundLoad)
  • Integration tests updated to xUnit v3 (TestContext.Current.CancellationToken pattern)

🤖 Generated with Claude Code

@GeorgeTsiokos GeorgeTsiokos requested review from a team as code owners March 13, 2026 10:37
@GeorgeTsiokos GeorgeTsiokos marked this pull request as draft March 13, 2026 11:47
@WhitWaldo
Copy link
Contributor

WhitWaldo commented Mar 13, 2026

Do note that I just merged a PR to support xUnit v3 so your unit/integration tests may need a refresh.

@GeorgeTsiokos @claude
Add optional parameter to configuration providers for non-blocking st…
f17d73f 
…artup

Configuration and secret store providers now accept an `optional` parameter
that allows applications to start without blocking on sidecar availability.
When optional, configuration loads in the background with resilient retry
logic that handles transient Dapr errors gracefully.

- Add `optional` parameter to AddDaprConfigurationStore and AddDaprSecretStore
- Harden LoadInBackgroundAsync to catch OperationCanceledException and
  DaprException separately, avoiding silent swallowing of programming errors
- Dispose CancellationTokenSource in both providers to prevent resource leaks
- Fix StringComparer inconsistency (InvariantCultureIgnoreCase → OrdinalIgnoreCase)
- Mark fields readonly in DaprConfigurationStoreProvider for consistency
- Restore UTF-8 BOM per .editorconfig and add missing trailing newlines
- Add deterministic tests using reload tokens instead of fragile Task.Delay

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: George Tsiokos <george@tsiokos.com>
@GeorgeTsiokos GeorgeTsiokos force-pushed the feature/optional-config-resilience branch from 7acf13b to 50765d7 Compare March 15, 2026 20:36
@GeorgeTsiokos @claude
Add integration tests for optional configuration provider resilience
d33da80 
Tests verify three scenarios using secretstores.local.file (no external deps):
- optional:true with delayed sidecar: config populates once sidecar starts
- optional:false (default): Build() blocks until sidecar ready, secrets populated
- Disposal during background load: exits cleanly with no hang

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: George Tsiokos <george@tsiokos.com>
@GeorgeTsiokos GeorgeTsiokos force-pushed the feature/optional-config-resilience branch from 50765d7 to d33da80 Compare March 15, 2026 20:37
@GeorgeTsiokos
Copy link
Author

GeorgeTsiokos commented Mar 15, 2026 •
edited
Loading

A few items I'm intentionally leaving as-is — noting them here for visibility:

Pre-existing catch (Exception) in streaming subscription loop (DaprConfigurationStoreProvider.cs) — The broad catch in the SubscribeConfiguration retry loop predates this PR. It's there to reconnect on any stream error (including grpc status errors that aren't DaprException). Left unchanged to avoid scope creep.

UnsubscribeConfiguration throwing inside catch block — Same pre-existing code, same reasoning.

Exponential backoff — The flat sidecarWaitTimeout delay between retries is intentional. The sidecar typically becomes available within its startup window; exponential backoff would add unnecessary complexity for this use case.

Splitting the try block in LoadInBackgroundAsync — The current structure is deliberately compact to keep the retry logic readable. Happy to revisit if the reviewer feels strongly.

Non-optional blocking-failure test for config store — Pre-existing gap (the non-optional path throws on WaitForSidecar timeout, but there's no unit test for it). Out of scope for this PR.

IsOptional source/builder layer test — The existing end-to-end integration tests in SecretStoreOptionalTests.cs cover this path. A pure unit test for the source layer would be redundant.

@GeorgeTsiokos GeorgeTsiokos marked this pull request as ready for review March 15, 2026 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add optional parameter to configuration providers

2 participants

@GeorgeTsiokos @WhitWaldo